-
Notifications
You must be signed in to change notification settings - Fork 21.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move while_preventing_writes from conn to handler #36469
Move while_preventing_writes from conn to handler #36469
Conversation
yield | ||
ensure | ||
@prevent_writes = original | ||
replica? || ActiveRecord::Base.connection_handler.prevent_writes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I never remember this but, does each model have its different connection handler or all models always share the ActiveRecord::Base
connection handler?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each type of connection has it's own handler depending on what you put in connects_to
.
Given the following:
class ApplicationRecord < ActiveRecord::Base
self.abstract_class = true
connects_to database: { writing: :primary, reading: :primary_replica }
end
class Person < ApplicationRecord
end
class AnimalsBase < ApplicationRecord
self.abstract_class = true
connects_to database: { writing: :animals, reading: :animals_replica }
end
class Dog < AnimalsBase
end
This will create 2 handlers a :writing
handler and a :reading
handler (aka role). Each of these handlers has 2 connections: one for application record (person) and one for animals base (dog). Because we use connected_to
to swap the handler out:
ActiveRecord::Base.connected_to(role: :reading) do
# do something on reading handler
end
means that ActiveRecord::Base.connection_handler.prevent_writes
will ask the handler whether it should prevent writes instead of the connection. Since connected_to
operates on the handler we can be confident it's passing the value down correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a test that demonstrates this is working correctly here https://github.com/rails/rails/pull/36469/files#diff-3ad790de5c6c5fbd6d956e443bcc0490R1579
@@ -986,15 +986,30 @@ def self.discard_unowned_pools(pid_map) # :nodoc: | |||
end | |||
end | |||
|
|||
attr_accessor :prevent_writes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need public attr_writer :prevent_writes
.
attr_accessor :prevent_writes | |
attr_reader :prevent_writes |
0656798
to
f53f9b2
Compare
If we put the `while_preventing_writes` on the connection then the middleware that sends reads to the primary and ensures they can't write will not work. The `while_preventing_writes` will only be applied to the connection which it's called on - which in the case of the middleware is Ar::Base. This worked fine if you called it directly like `OtherDbConn.connection.while_preventing_writes` but Rails didn't have a way of knowing you wanted to call it on all the connections. The change here moves the `while_preventing_writes` method from the connection to the handler so that it can block writes to all queries for that handler. This will apply to all the connections associated with that handler.
f53f9b2
to
cd881ab
Compare
…es-to-handler Move while_preventing_writes from conn to handler
Move while_preventing_writes from conn to handler rails/rails#36469
If we put the
while_preventing_writes
on the connection then themiddleware that sends reads to the primary and ensures they can't write
will not work. The
while_preventing_writes
will only be applied to theconnection which it's called on - which in the case of the middleware is
Ar::Base.
This worked fine if you called it directly like
OtherDbConn.connection.while_preventing_writes
but Rails didn't have away of knowing you wanted to call it on all the connections.
The change here moves the
while_preventing_writes
method from theconnection to the handler so that it can block writes to all queries for
that handler. This will apply to all the connections associated with
that handler.
cc/ @matthewd @tenderlove @jhawthorn @rafaelfranca